Skip to content

Filters::spacelessHtml() Add support for use <script> as link to JS - #196

Closed
janbarasek wants to merge 4 commits into
nette:masterfrom
janbarasek:patch-1
Closed

Filters::spacelessHtml() Add support for use <script> as link to JS#196
janbarasek wants to merge 4 commits into
nette:masterfrom
janbarasek:patch-1

Conversation

@janbarasek

Copy link
Copy Markdown
  • bug fix
  • BC break? yes

In case of block y generate too long code:

{block x|strip}
	Lorem ipsum dolor sit amet,
	<script src="main.js"></script>
	consectetur adipiscing elit.
{/block}

------

{block y|strip}
	Lorem ipsum dolor sit amet,
	<script src="main.js"
			title="value"
	></script>
	consectetur adipiscing elit.
{/block}

Old return:

Lorem ipsum dolor sit amet, <script src="main.js"></script> consectetur adipiscing elit.
------

Lorem ipsum dolor sit amet, <script src="main.js"
			title="value"
	></script> consectetur adipiscing elit.

New return:

Lorem ipsum dolor sit amet, <script src="main.js"></script> consectetur adipiscing elit.
------

Lorem ipsum dolor sit amet, <script src="main.js" title="value"></script> consectetur adipiscing elit.

I think this feature is absolutely safe, because if you have empty <script> body you can safely replace repeating whitespaces to single whitespace.

Real case in real website:

Snímek obrazovky 2019-08-31 v 17 56 23

Thanks.

$s = rtrim($s);
}
return preg_replace_callback(
$return = (string) preg_replace_callback( // Other cases

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why (string)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because PhpStorm hint to me new type can be returned in future. For example null in case of error.

$return = (string) preg_replace_callback( // Other cases
'#[ \t\r\n]+|<(/)?(textarea|pre|script)(?=\W)#si',
function ($m) use (&$strip) {
static function (array $m) use (&$strip): string {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why static?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP does not have to create a new instance every time.

$s
);
$return = (string) preg_replace_callback( // <script> for include JS file
'/<script\s*([^>]+?)>(?:\s*)<\/script>/',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this regexp work with input like <script title="> <\/script>"> ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This input is not supported now. I'll try to come up with a better solution.

@dg
dg force-pushed the master branch 3 times, most recently from 1a297fd to 8fb996f Compare September 18, 2019 14:37
@dg
dg force-pushed the master branch 4 times, most recently from 4d5343b to 6edda93 Compare November 20, 2019 11:58
@dg
dg force-pushed the master branch 3 times, most recently from 090695a to dba2a60 Compare December 17, 2019 04:36
@dg
dg force-pushed the master branch 10 times, most recently from d3ff96d to 47c1926 Compare December 23, 2019 00:58
@dg
dg force-pushed the master branch 2 times, most recently from 942f9b1 to 753a2fc Compare January 3, 2020 22:38
@dg
dg force-pushed the master branch 4 times, most recently from 12b410d to 3d14ad7 Compare January 28, 2020 17:47
@dg
dg force-pushed the master branch 4 times, most recently from 8f69b66 to 682b403 Compare February 12, 2020 14:28
@dg
dg force-pushed the master branch 6 times, most recently from b340768 to 8af4bf1 Compare February 28, 2020 13:11
@dg
dg force-pushed the master branch 14 times, most recently from c3d3812 to ca328ce Compare March 10, 2020 12:21
@dg
dg force-pushed the master branch 2 times, most recently from a9010bd to 55326a1 Compare March 12, 2020 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants